Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flexible declaration styling #9023

Merged
merged 22 commits into from
Apr 12, 2021
Merged

Conversation

jakobandersen
Copy link
Contributor

@jakobandersen jakobandersen commented Mar 20, 2021

Feature or Bugfix

  • Feature
  • Refactoring

Purpose

Attempt to resolve a foundational chunk of #4289, building on #7145, #6990, #4390.

Examples of difference in HTML rendering

Current: https://jakobandersen.github.io/ex_sphinx_decl_styling/master/
With this: https://jakobandersen.github.io/ex_sphinx_decl_styling/with_9023/

For seeing the differences for the Python domain (change to monospace font) see e.g., extdev/appapi.html.
For the difference in styling with colouring in the C++ domain see e.g., usage/restructuredtext/domains.html#_CPPv46MyList.

Additional example of C++ differences

Current:
master
With this PR:
with_9023

Details

  • Add more nodes derived from addnodes.desc_sig_element,
    and document them as being the leaf-nodes of signatures.
    That is, they only contain text, no other nodes.
  • Document the nodes desc, desc_signature, desc_signature_line, and desc_content
    as the nodes that form the top levels of object descriptions.
  • Document the remaining desc_nodes as being high-level structuring nodes for signatures.
  • Document which classes are always in the desc_ nodes.
  • Revamp the C++ domain to use the new nodes, and never nodes.Text nodes.
  • Restructure the CSS in the basic theme to group by this node classification.
  • Move classes on nodes to addnodes from the HTML and HTML5 writers.
  • Introduce a new top-level desc_inline node for signature fragments in text.
    This is not for xrefs, but for roles like cpp:expr which contain arbitrary expressions or types.
  • Add new classes on the two top-level signature-containing nodes desc_signature and desc_inline.
    Both get sig the domain name, and in addition sig-object and sig-inline respectively.
    While desc contains the domain, this also has arbitrary descriptions as decendants,
    so you can have a py:function with a description that contains a cpp:expr.
    If you want to style, e.g., keywords differently in the two languages, you can now select
    .sig.py .k and .sig.cpp .k without them overlapping.
  • HTML: Change desc_name and desc_addname from using a code tag to a span,
    and instead make all .sig use monospace as font-family consistently.
  • Add styling to C++ signatures based on the IntellliJ theme.

Next steps, perhaps for future PRs

  • Only the basic theme has been changed. The rest should be checked. (done)
  • @jfbu, the Latex implementation of desc_inline is quick copy paste. The compiled PDF looks fine to me, but please check, e.g., with something like :cpp:expr:`a + b`
  • There are some more desc_ nodes perhaps should be looked at:
    desc_type, desc_returns, desc_parameterlist, desc_parameter, desc_optional, and desc_annotation.
    I suggest doing this in a separate PR.
  • As discussed in Flexible styling of declarations #4289 it may be nice to also have flexible styling in Latex. As the current classes are way fewer than in Pygments I don't think a direct link to their styles is relevant. I suggest doing this in a separate PR if those using the Latex output would like it, but it would be nice to get feedback already now if there is a problem with the design suggested here (ping @jfbu ?).
  • Convert the C domain to use the new nodes. (done)
  • Convert other domains to use the new nodes?

@jakobandersen jakobandersen added domain domains:cpp type:enhancement enhance or introduce a new feature labels Mar 20, 2021
@jakobandersen jakobandersen force-pushed the decl_styling branch 2 times, most recently from 4bfefc5 to b16cb6c Compare March 25, 2021 19:50
@jakobandersen jakobandersen changed the title [WIP] Flexible declaration styling Flexible declaration styling Mar 25, 2021
@jakobandersen
Copy link
Contributor Author

I think it is now in a potentially mergeable state, ready to review.

sphinx/addnodes.py Outdated Show resolved Hide resolved
sphinx/addnodes.py Outdated Show resolved Hide resolved
sphinx/addnodes.py Show resolved Hide resolved
sphinx/transforms/post_transforms/__init__.py Outdated Show resolved Hide resolved
sphinx/writers/manpage.py Show resolved Hide resolved
sphinx/addnodes.py Show resolved Hide resolved
@tk0miya
Copy link
Member

tk0miya commented Apr 4, 2021

LGTM with nits!

@jakobandersen jakobandersen mentioned this pull request Apr 7, 2021
@jakobandersen
Copy link
Contributor Author

LGTM with nits!

Thanks! It should be ready for a second round now.

As a general point: what is your opinion on how to use SigElementFallbackTransform? If we should minimize the use of it, I suggest we add warnings whenever it triggers so builders can become aware of missing implementations.

@tk0miya tk0miya modified the milestones: 4.0.0, 4.1.0 Apr 11, 2021
@tk0miya
Copy link
Member

tk0miya commented Apr 11, 2021

As a general point: what is your opinion on how to use SigElementFallbackTransform? If we should minimize the use of it, I suggest we add warnings whenever it triggers so builders can become aware of missing implementations.

Ideally, all writers should support all of the new nodes. But it increases the maintenance cost of the 3rd party builders. So I'm afraid that the warnings kill their deprecation because of the lack of maintenance. So -0 for emitting warnings.

@jakobandersen
Copy link
Contributor Author

So I'm afraid that the warnings kill their deprecation because of the lack of maintenance.

Good point, as noted in the review comment, I'll ensure that at least the writers bundled with Sphinx only needs the fallback for the desc_sig_element nodes. After that I'll merge into 4.0.0, ok?

@tk0miya
Copy link
Member

tk0miya commented Apr 11, 2021

Yes. I'll review this again tomorrow (or the next day). After that, let's merge this into 4.0.x branch.

@tk0miya
Copy link
Member

tk0miya commented Apr 12, 2021

LGTM again. +1 for merging if fallback-transform supports the desc_inline node.

@jakobandersen
Copy link
Contributor Author

Thanks for the reviews! I'll rebase to 4.0.x and merge.

@jakobandersen jakobandersen changed the base branch from master to 4.0.x April 12, 2021 17:08
@jakobandersen jakobandersen merged commit 8954770 into sphinx-doc:4.0.x Apr 12, 2021
@jakobandersen jakobandersen deleted the decl_styling branch April 12, 2021 20:05
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants